Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TableSasBuilder pattern changes #11790

Merged

Conversation

christothes
Copy link
Member

@christothes christothes commented May 5, 2020

Summary

The focus of these changes is to improve the discoverability and usability of generating Shared Access Signature tokens for the Table service. The primary change is the introduction of a new GetSasBuilder method on the TableClient and some improved defaults to the TableSasBuilder ctor to help guide developers to success.

@christothes christothes added Client This issue points to a problem in the data-plane of the library. Tables labels May 5, 2020
@christothes christothes requested a review from tg-msft May 5, 2020 01:31
@christothes christothes self-assigned this May 5, 2020
@christothes christothes changed the title initial draft of SasBuilder pattern changes TableSasBuilder pattern changes May 5, 2020
@christothes christothes marked this pull request as ready for review May 5, 2020 14:58
Copy link
Member

@tg-msft tg-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good - just a few comments.

Comment on lines 236 to 237
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
public string Version { get { throw null; } set { } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably just delete the Version property. The Storage folks decided they'd only support latest and we might as well follow suit unless we explicitly decide otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll make it internal so that we can still set it from the latest version defined in TableClientOptions

@christothes christothes requested a review from JeffreyRichter May 6, 2020 18:34
Copy link
Member

@tg-msft tg-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@christothes christothes merged commit 8682c80 into Azure:master May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Tables
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants